Speed up cache freshness checks on large archives#266
Conversation
Even with a fully warm cache, launching the TUI on a multi-thousand- file archive spent seconds re-verifying freshness. Profiling against a real 2GB / 5,381-item ~/.claude/projects copy showed the file stats were never the problem (~5ms for 1,830 files); the cost was per-file SQLite connections, per-file sidecar dir probes, and a dead query. Four changes, all in cache.py: - get_modified_files(): batched equivalent of per-file is_file_cached() — one query fetches every cached row for the project (one connection open instead of one per file, composing with batch() from #251), and one scandir per parent directory rules out subagent-sidecar fingerprints (#213) so the fingerprint scan only runs for files that actually have a <stem>/ dir (~154 of 1,830 in the test archive). Semantics verified identical to the per-file path on real data. - get_cached_project_data(): drop the per-file "SELECT DISTINCT session_id FROM messages" loop feeding CachedFileInfo.session_ids — the field was written but never read anywhere (production, tests, or docs), and cost N queries over the largest table on every call. Field removed from the model. - get_cached_project_data(): fetch working_directories on the same connection instead of opening a second one per call. - _init_database(): memoise the migration check per DB path per process — the TUI project selector constructs a CacheManager per row on every resize, each re-paying ~2ms of migration checks. An exists() guard keeps --clear-cache's DB deletion re-migrating. Measured on the 2GB archive (63 projects, warm cache, best of 3), before -> after on current main: TUI launch, one project (453 files): 646ms -> 12ms (54x) TUI project selector (63 projects): 1190ms -> 276ms (4x) Full-archive freshness scan: 4652ms -> 358ms (13x) Approaches considered and rejected: directory-mtime fast path (unsound — appends don't touch dir mtime; and stats were never the bottleneck), hash/probabilistic checks (same reason), parallelism (unnecessary at these timings). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors the SQLite cache module to memoize migration checks per database path, centralize freshness validation, batch cached-file metadata queries for freshness scanning, remove ChangesCache freshness and migration memoization
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
claude_code_log/cache.py (1)
912-990: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFreshness logic duplicated between
is_file_cached()andget_modified_files().Both methods independently re-implement the same mtime-tolerance check and the NULL-fingerprint acceptance rule (Lines 616-629 vs. 955-989). The batched version is a legitimate optimization (scandir pre-filter, single query), but any future change to the freshness rules now needs to be applied in two places, risking drift.
Consider extracting the pure decision (
row,source_mtime,current_fp) →boolinto a small shared helper that both methods call, keeping the batching/scandir optimization only inget_modified_files().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@claude_code_log/cache.py` around lines 912 - 990, The freshness decision logic is duplicated between is_file_cached() and get_modified_files(), so extract the pure cache-validity check into a shared helper that takes the cached row, current source_mtime, and current subagents fingerprint and returns whether the file is fresh. Update both is_file_cached() and get_modified_files() to call that helper, while keeping get_modified_files()’s batching and scandir optimization intact; use the existing symbols is_file_cached, get_modified_files, and subagents_fingerprint to locate the shared rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@claude_code_log/cache.py`:
- Around line 912-990: The freshness decision logic is duplicated between
is_file_cached() and get_modified_files(), so extract the pure cache-validity
check into a shared helper that takes the cached row, current source_mtime, and
current subagents fingerprint and returns whether the file is fresh. Update both
is_file_cached() and get_modified_files() to call that helper, while keeping
get_modified_files()’s batching and scandir optimization intact; use the
existing symbols is_file_cached, get_modified_files, and subagents_fingerprint
to locate the shared rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d629b424-72d0-4446-8587-ec40ccb935e0
📒 Files selected for processing (2)
claude_code_log/cache.pydev-docs/application_model.md
Speed up cache freshness checks 13–54× on large archives (#12)
Even with a fully warm cache, launching the TUI on a big archive spent seconds
re-verifying freshness. This PR attacks the check itself, benchmarked against a
real 2 GB / 5,381-item copy of
~/.claude/projects(63 projects, ~1,830session JSONLs, 1,603 sessions).
Results
Warm cache, best of 3, before → after on current
main(which alreadyincludes #251):
The
--tuilaunch path runs the freshness check twice (once in_launch_tui_with_cache_check, again inSessionBrowser.load_sessions), sothe user-visible launch delay was ~1.3 s and is now ~25 ms.
What was actually slow
The file stats were never the problem — stat'ing all 1,830 files takes ~5 ms,
and even a full recursive walk of the 2 GB tree is ~150 ms. The measured costs:
get_modified_files()calledis_file_cached()per file, each opening a fresh connection (~40× the costof the query it runs). Speed up the SQLite metadata-cache build (connection reuse + synchronous=NORMAL) #251 fixed this pattern for cache builds via
batch(), but the freshness-check paths don't run inside a batch.subagents_fingerprint()(Support hierarchies of agents #213) does anis_dir()probe + glob per file, ~160 ms across the archive — almost allfor files that can't have sidecars.
get_cached_project_data()ranSELECT DISTINCT session_id FROM messages WHERE file_id = ?per cachedfile to fill
CachedFileInfo.session_ids— a field nothing reads(production code, tests, or docs). This is what made the project selector
slow.
CacheManagerconstruction re-ran themigration check (~2 ms); the TUI selector constructs one per project row,
on every resize.
Changes (all in
cache.py)get_modified_files()— one query fetches every cached row forthe project (one connection open; zero extra inside a
batch()scope), andone
os.scandirper parent directory determines which files even canhave sidecars, so the fingerprint scan only runs for those (~154 of 1,830
here). Semantics are identical to the per-file path — verified below.
CachedFileInfo.session_idsand its N-queries-per-call.(Measured alternatives: single
GROUP BYquery = 209 ms, per-file lookupson a shared connection = 131 ms, not doing it = 4 ms.)
get_cached_project_data()fetchesworking_directorieson the sameconnection instead of opening a second one per call.
exists()guard so--clear-cache's DB deletion still re-migrates.Connection lifecycle is untouched: connection-per-call stays the default
(the Windows-safe behaviour pinned by #251's integrity tests), and the new
code composes with
batch().Approaches considered and rejected
check — appending to a JSONL doesn't update the directory mtime, so it can
only detect create/delete/rename. And since per-file stats cost ~5 ms
total, it would save nothing meaningful.
were never the bottleneck; probabilistic is also non-deterministic.
could still help the one-time initial parse (~400 s for the full 2 GB),
but that's a rare event and separate from this issue.
Verification
new sidecar detected (including for a session with no pre-existing
<stem>/dir — the case the scandir shortcut could plausibly havebroken), new file detected, and full parity with per-file
is_file_cached()both outside and inside abatch()scope.lifecycle integrity tests);
ruff checkandty checkclean.Numbers above are from a Linux dev container; absolute values will differ on
macOS but the ratios should hold, since the fix removes work rather than
speeding it up.
Summary by CodeRabbit